Skip to content

Remove sequence diagram image in the yaml and instead refer to ICM#213

Merged
bigludo7 merged 4 commits intomainfrom
fix207&209
Aug 27, 2025
Merged

Remove sequence diagram image in the yaml and instead refer to ICM#213
bigludo7 merged 4 commits intomainfrom
fix207&209

Conversation

@bigludo7
Copy link
Collaborator

@bigludo7 bigludo7 commented Aug 22, 2025

What type of PR is this?

Add one of the following kinds:

  • bug
  • correction
  • enhancement/feature
  • cleanup
  • documentation
  • subproject management
  • tests

What this PR does / why we need it:

Remove sequence diagram image in the yaml (it was not up to date and this redundant with ICM documentation) - instead refer ICM

Which issue(s) this PR fixes:

Fixes #207 #209

Special notes for reviewers:

Changelog input

 release-note
- Remove sequence diagram in the yaml and point to ICM documentation

Additional documentation

This section can be blank.

docs

@github-actions
Copy link

github-actions bot commented Aug 22, 2025 •
edited
Loading

🦙 MegaLinter status: ✅ SUCCESS

Descriptor Linter Files Fixed Errors Elapsed time
✅ ACTION actionlint 2 0 0.0s
✅ API spectral 1 0 1.5s
✅ GHERKIN gherkin-lint 2 0 0.68s
✅ REPOSITORY git_diff yes no 0.0s
✅ REPOSITORY secretlint yes no 0.65s
✅ YAML yamllint 1 0 0.36s

See detailed report in MegaLinter reports

MegaLinter is graciously provided by OX Security

@bigludo7 bigludo7 changed the title Remove sequence diagram image in the yaml to refer ICM Remove sequence diagram image in the yaml and instead refer to ICM Aug 22, 2025
AxelNennker
AxelNennker requested changes Aug 25, 2025
View reviewed changes
Copy link
Collaborator

@AxelNennker AxelNennker left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Changed the link from pointing to "main" to pointing to latest release.
  • Changed the note to only apply to CIBA only, as JWT Bearer Flow is defined without user interaction.

diegogonmar
diegogonmar reviewed Aug 25, 2025
View reviewed changes
Copy link
Contributor

@diegogonmar diegogonmar left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think content is a little bit mixed now because the section Sequence Diagram renamed to Implementation details previously included flow and notes about AuthCode flow. Now mixes sentences about CIBA/JWT-Bearer with AuthCode (e.g.: about prompt parameter).

I propose following approach instead:

  • Remove full section Sequence Diagram

  • In section Authentication Request with a temporary token

    • Include the sentence saying that API provider guarantees there is no user interaction. No strong opinion if needeed to indicate that this applies to CIBA only.
    • Include links to point to detailed flows.

  • In section Authentication Request without a temporary token

    • Include the details about prompt=none. This is about authentication request so makes sense to have it in this section.

I doubt what to do with the note about how the phone number is retrieved. It may fit in Resources and operations overview section

If you think this changes many things and prefer to maintain a separated section it's fine also, but these clarifications and differentiation with AuthCode are needed anyway IMO.

@bigludo7
Copy link
Collaborator Author

bigludo7 commented Aug 25, 2025

Thanks a lot @AxelNennker and @diegogonmar for your comment.

I've a new proposal. I understood that the point guaranteeing that there is no user interaction is the crucial one so I move directly under the the Authentication Request part (whatever we use of not a temporary token did not change the fact that no user interaction - right?).
I changed the sentence to the Axel proposed one.

I removed the note about he note about how the phone number is retrieved. After all this yaml for the app developer and she/he did not care as it is our fabric recipe.

Looking for your feedbacks.

diegogonmar
diegogonmar approved these changes Aug 25, 2025
View reviewed changes
Copy link
Contributor

@diegogonmar diegogonmar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think its clearer and simpler now, thanks @bigludo7.
LGTM

@AxelNennker
Copy link
Collaborator

AxelNennker commented Aug 25, 2025

As the PR covers more than removing or replacing the sequence diagrams already, that is more, than #207 and #209, I suggest recommending JWT Bearer Flow over CIBA.

Replace this:

The API Consumer sends the temporary token to their backend which either:
- Sends a CIBA Authentication Request, as described in the current release CAMARA APIs Access and User Consent Management, with a parameter login_hint=operatortoken:<temporary token>.
- Or sends a JWT-Bearer token request as described in CAMARA APIs Access and User Consent Management, with the TS.43 token in the sub claim of the JWT assertion with the format "operatortoken:<temporary token>".

By this:

From the mobile device the TS.43 token is send to the API consumer's backend.
It is recommended that the backend sends a JWT-Bearer token request to the API provider's authorization server. JWT Bearer Flow is defined with no user interaction and overall fewer network requests are sent compared to CIBA.
Alternatively, the backend sends a CIBA Authentication Request.
The used flow is determined at onboarding time.
Both flows, JWT Bearer Flow and CIBA are described in CAMARA APIs Access and User Consent Management.

@diegogonmar
Copy link
Contributor

diegogonmar commented Aug 26, 2025

As the PR covers more than removing or replacing the sequence diagrams already, that is more, than #207 and #209, I suggest recommending JWT Bearer Flow over CIBA.

Replace this:

The API Consumer sends the temporary token to their backend which either:

By this:

From the mobile device the TS.43 token is send to the API consumer's backend.
It is recommended that the backend sends a JWT-Bearer token request to the API provider's authorization server. JWT Bearer Flow is defined with no user interaction and overall fewer network requests are sent compared to CIBA.
Alternatively, the backend sends a CIBA Authentication Request.
The used flow is determined at onboarding time.
Both flows, JWT Bearer Flow and CIBA are described in CAMARA APIs Access and User Consent Management.

This PR is cleanup/documentation. Recommending a flow over another is something different. Needs an issue and a group agreement, and further I don't think it's something to be covered when moving from RC to public. Therefore, I don't agree with using this PR for that.

@bigludo7
Copy link
Collaborator Author

bigludo7 commented Aug 26, 2025

I tend to agree with @diegogonmar and remind that this PR objective is only to remove the sequence diagram.

Could we

  • Move forward with this PR in order to allow me to prepare M4 (deadline is this week) except if any of you see a strong blocker.
  • Create an issue either here or in ICM to cover @AxelNennker proposal.

@bigludo7 bigludo7 merged commit 243cfdb into main Aug 27, 2025
2 checks passed
@bigludo7 bigludo7 deleted the fix207&209 branch August 27, 2025 06:19
This was referenced Aug 27, 2025
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AxelNennker AxelNennker AxelNennker approved these changes

@fernandopradocabrillo fernandopradocabrillo fernandopradocabrillo left review comments

@shilpa-padgaonkar shilpa-padgaonkar Awaiting requested review from shilpa-padgaonkar

@maxl2287 maxl2287 Awaiting requested review from maxl2287

+1 more reviewer

@diegogonmar diegogonmar diegogonmar approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Referenced UML content is not up-to-date

4 participants

@bigludo7 @AxelNennker @diegogonmar @fernandopradocabrillo